Conversation
Change the default manifest source for non-Deno (Bolt) projects from ManifestSourceRemote to ManifestSourceLocal, aligning their behavior with Deno projects.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #396 +/- ##
==========================================
- Coverage 67.90% 67.90% -0.01%
==========================================
Files 218 218
Lines 18085 18050 -35
==========================================
- Hits 12281 12256 -25
+ Misses 4651 4640 -11
- Partials 1153 1154 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mwbrooks
left a comment
There was a problem hiding this comment.
Some words for the kind reviewers!
cmd/app/link.go
Outdated
| // App Manifest section | ||
| manifestSource, _ := clients.Config.ProjectConfig.GetManifestSource(ctx) | ||
| if !manifestSource.Exists() { | ||
| manifestSource = config.ManifestSourceLocal |
There was a problem hiding this comment.
note(out-of-scope): We should have the default manifest source return by a common method instead of manually setting it in each spot.
There was a problem hiding this comment.
🪬 question: I notice that without a source attribute in configs we default to a local manifest. Is this something we can favor instead of writing settings to projects?
👾 note: I realize a saved value in each project settings file means less confusion toward defaults, but IMHO we might have paths toward extending the "local" manifest experience adjacent.
slack-cli/internal/config/project.go
Lines 147 to 171 in 8e4c0aa
There was a problem hiding this comment.
Good question. My feeling is that - for now - we should explicitly write to the config.json to ensure that a project functions the way it's expected even if the CLI changes it's defaults.
However, I see where you're coming from. I also know we juggle some challenges with config.json in templates (e.g. Project ID). Perhaps your ideas would have address those?
| ExpectedError: slackerror.New(slackerror.ErrAppNotFound), | ||
| }, | ||
| "accepting manifest source prompt should save information about the provided deployed app": { | ||
| "links app when manifest source is local": { |
There was a problem hiding this comment.
note: Test linking an app when the manifest source is local (manifest file).
| }, | ||
| }, | ||
| "manifest source prompt should display for GBP apps with local manifest source": { | ||
| "links app when manifest source is remote": { |
There was a problem hiding this comment.
note: Test linking an app when the manifest source is remote (app settings).
| notice = "Manifest values for this app are overwritten on reinstall" | ||
| default: | ||
| notice = "The manifest on app settings has been changed since last update!" | ||
| notice = style.Yellow("The manifest on app settings has been changed since last update") |
There was a problem hiding this comment.
note: I'm open to other formatting (Red?) but I think we should make this pop and stand out in some way.
There was a problem hiding this comment.
🌟 praise: I like yellow: it matches error codes and isn't confusing with green! But we might continue these discussions in a style guide soonish-
cmd/app/link.go
Outdated
| // App Manifest section | ||
| manifestSource, _ := clients.Config.ProjectConfig.GetManifestSource(ctx) | ||
| if !manifestSource.Exists() { | ||
| manifestSource = config.ManifestSourceLocal |
There was a problem hiding this comment.
🪬 question: I notice that without a source attribute in configs we default to a local manifest. Is this something we can favor instead of writing settings to projects?
👾 note: I realize a saved value in each project settings file means less confusion toward defaults, but IMHO we might have paths toward extending the "local" manifest experience adjacent.
slack-cli/internal/config/project.go
Lines 147 to 171 in 8e4c0aa
cmd/app/link.go
Outdated
| if !manifestSource.Exists() { | ||
| manifestSource = config.ManifestSourceLocal | ||
| } |
There was a problem hiding this comment.
| if !manifestSource.Exists() { | |
| manifestSource = config.ManifestSourceLocal | |
| } |
🪬 quibble: Related to the comment above, this might be alright to remove?
| notice = "Manifest values for this app are overwritten on reinstall" | ||
| default: | ||
| notice = "The manifest on app settings has been changed since last update!" | ||
| notice = style.Yellow("The manifest on app settings has been changed since last update") |
There was a problem hiding this comment.
🌟 praise: I like yellow: it matches error codes and isn't confusing with green! But we might continue these discussions in a style guide soonish-
| clients.IO.PrintInfo(ctx, false, "%s", style.Sectionf(style.TextSection{ | ||
| Emoji: "books", | ||
| Text: "App Manifest", | ||
| Secondary: []string{ | ||
| "Manifest source is " + style.Highlight(manifestSource.Human()), | ||
| "Manifest source is configured in " + style.Highlight(configPath), | ||
| }, | ||
| })) |
There was a problem hiding this comment.
🧭 praise: This is a nice callout to new learnings in this experience! I'm not certain how familiar the local|remote manifest concept is to exploring developers.
There was a problem hiding this comment.
It crossed my mind as well and we don't have formal documentation that we can point toward. I figured I'd leave it as a future task for now.
…oks-default-bolt-manifest-source-local-warnings
Co-authored-by: Eden Zimbelman <eden.zimbelman@salesforce.com>
|
Thanks for the review and suggested improvements @zimeg! I think this is a step forward and we may have some new ideas on improving the project's |
Changelog
Summary
This pull request is a little spicy 🌶️ because remove speed bumps and reduces friction for developers to allows apps from App setting to be managed by the Slack CLI.
See Changelog section for a detailed breakdown of the changes.
Motivation
The motivation for these changes is 💬 developer feedback. A consistent theme of questions from developers is how to have the Slack CLI manage and use the
manifest.jsonthat ships with all of our sample apps (sensible question). We often need to explain how to manually update.slack/config.json → manifest.source: "local"for developers to get the experience they want.Our concern for embracing the project's manifest file is that it can overwrite the manifest on App Settings. This could be a shocking and difficult experience, if it happens unexpectedly. Thankfully, the Slack CLI's overwrite protection has been reliable for the past year and this change leans more heavily on it.
Preview
Linking an App ID from App Setting
Currently, when Linking an App ID from App Setting, the developer is forced to switch the manifest source to be App Setting (remote). If they decline the confirmation prompt, then they cannot link the app. A workaround that developers have taken is to accept the confirmation and then manually change the manifest source back to the manifest file:
.slack/config → manifest.source: "local".Now, the manifest source is not changed - it can remain local (manifest file) or remote (app settings). We rely on the manifest overwrite protection to warn developers when there are changes on App Settings that are not in the manifest file.
Expand to see a video of the current behaviour
2026-03-13-app-link-current.mov
2026-03-13-app-link-new.mov
Overwriting App Settings Warning
We made minor wording and appearance changes to clarify that the app settings manifest will be overwritten by the manifest file.
2026-03-13-overwrite-manifest-current.mov
2026-03-13-overwrite-manifest-new.mov
Test Steps
Requirements